Skip to content

server: Cleanup external address discovery.#3640

Open
davecgh wants to merge 3 commits intodecred:masterfrom
davecgh:server_external_addr_discovery
Open

server: Cleanup external address discovery.#3640
davecgh wants to merge 3 commits intodecred:masterfrom
davecgh:server_external_addr_discovery

Conversation

@davecgh
Copy link
Copy Markdown
Member

@davecgh davecgh commented Mar 6, 2026

This requires #3638.

This current code that handles the external server address discovery is rather difficult to follow because it is poorly named and interspersed with other code that is not related.

This refactors the code to make it better separated and easier to follow. It also switches the main data structure that limits the number of addresses to use an LRU instead.

While refactoring this, I noticed there is room for improvement in terms of the logic as well. However, in order to keep the changes easier to review for correctness, this does not contain any notable overall logic changes and limits the changes to refactors and cleanup.

@davecgh davecgh added this to the 2.2.0 milestone Mar 6, 2026
@davecgh davecgh force-pushed the server_external_addr_discovery branch from 221a417 to 1bb946e Compare March 6, 2026 18:31
@davecgh davecgh force-pushed the server_external_addr_discovery branch from 1bb946e to f7d3721 Compare March 17, 2026 06:10
davecgh added 3 commits April 6, 2026 23:14
This modifies the lifecycle of peers to use the more modern Run pattern
that based on contexts.

In particular, this replaces the Start and WaitForDisconnect methods
with a single method named Run and arranges for it to block until the
provided context is cancelled or the peer is disconnected.  This is more
flexible for the caller since it can easily turn blocking code into
async code while the reverse is not true.

The new Run method waits for all goroutines that it starts to shutdown
before returning to help ensure an orderly shutdown.

Since all exported methods that send messages to the various groroutines
via channels already select across the quit channel which is closed when
the peer disconnects, the peer is now forcibly disconnected when the
context is cancelled.

This approach allows the flexibility for callers to use any combination
of manually disconnecting peers via the Disconnect method and allowing
them to automatically be disconnected when the context is cancelled.

It also updates the server code accordingly.
Currently, the whitelist and banning detection splits the host and port
multiple times since the address previously hadn't been parsed yet.

However, it is no longer necessary since the parsed address is now
available as soon as the peer first connects.

This updates the detection funcs to take the parsed address directly.
This current code that handles the external server address discovery is
rather difficult to follow because it is poorly named and interspersed
with other code that is not related.

This refactors the code to make it better separated and easier to
follow.  It also switches the main data structure that limits the number
of addresses to use an LRU instead.

While refactoring this, I noticed there is room for improvement in terms
of the logic as well.  However, in order to keep the changes easier to
review for correctness, this does not contain any notable overall logic
changes and limits the changes to refactors and cleanup.
@davecgh davecgh force-pushed the server_external_addr_discovery branch from f7d3721 to 10a916d Compare April 7, 2026 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant